Fix initializing Macronix QSPI flashes with only 2 status registers #444
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Testing code for the Arduino Portenta, I noticed that its QSPI flash (MX25L12833F) was failing to init. Digging a bit deeper, it appears that it's hitting an assert failure in
QSPIFBlockDevice::_enable_fast_mode(). This function is trying to enable fast mode using the 3rd status register, but the flash only has 2 status registers, so it dies.As this comment explains, some flashes need this fast mode to hit their full clock frequency. However, MX25L12833F does not have a third status register and does not need fast mode -- it can run at up to 84MHz for a QSPI read without any special configuration (as far as I can tell from the datasheet).
The issue is that the logic in
QSPIFBlockDevice::_handle_vendor_quirks()always sets_needs_fast_modeto true for Macronix devices, even if theQSPI_MACRONIX_NUM_STATUS_REGISTERoption is set to 2. This makes it impossible to setQSPI_MACRONIX_NUM_STATUS_REGISTERto 2, because you will always get an assert failure.To fix this, I tweaked the logic so that
_needs_fast_modeis only set to 2 if we have 3 or more status registers, so we (think we) have the needed flag. Otherwise, we don't try to enable fast mode. I am not sure that this will work for all Macronix flashes, but it seems to work for the ones already supported and the MX25L12833F.Update: Made a second fix after some more testing. The comment says that 4 byte mode does not work with Macronix devices, but the code did not actually follow through with that by setting
_attempt_4_byte_addressingto false. Without this setting, testing on Arduino Portenta, the flash inits fine but data programmed to it is not actually saved and/or cannot be read back. So I went ahead and added_attempt_4_byte_addressing = false, and it completely fixed the issue.Impact of changes
QSPI_MACRONIX_NUM_STATUS_REGISTERset to 2 without hitting an assert failure.Migration actions required
Documentation
None
Pull request type
Test results
Able to init the flash on an Arduino Portenta with this change.